Skip to content

GHI #112, #119 - Fix API data pipeline & add altitude tape [FV/typescript-rewrite] - #116

Open
ETSells wants to merge 6 commits into
mainfrom
integration/state-estims-telemetry
Open

GHI #112, #119 - Fix API data pipeline & add altitude tape [FV/typescript-rewrite]#116
ETSells wants to merge 6 commits into
mainfrom
integration/state-estims-telemetry

Conversation

@ETSells

@ETSells ETSells commented Jul 5, 2026

Copy link
Copy Markdown
Member

Issues: #112, #119 (relates to: #44, #101)

Parent: SunDevilRocketry/Flight-Computer-Firmware#301

Description

Adds an altitude tape widget to the right of the renderer, fixes the new data pipeline from the API, adds the remaining data readouts to the sensor readings widget, and fixes some responsiveness issues.

image

Due to a lack of response on the original and the need to build on existing changes, additional features have been added on top of this PR:

  1. Add display unit converters for altitude and acceleration
    a. Ideally, we'll include settings page linkage once that exists, but for now this already helps unify our view/presenter framework a bit.
    b. All unit converters implement an interface that defines the base functions, so polymorphism is possible.
    c. The altitude converter also includes a ground reference "QFE" mode in addition to the usual sea-level reference "QNH" mode.
image

NOTE: Builds fail due to an issue that predates this PR and should be fixed by the owner of the target branch. No new errors are generated by this PR, and npm run dev still works.

▲ Next.js 16.2.9 (Turbopack)

  Creating an optimized production build ...
✓ Compiled successfully in 1823ms
  Running TypeScript  .Failed to type check.

./src/utils/mock.ts:1:16
Type error: Invalid module name in augmentation. Module 'papaparse' resolves to an untyped module at 'C:/Users/etsel/SDR/dashboard/node_modules/papaparse/papaparse.js', which cannot be augmented.

> 1 | declare module "papaparse";
    |                ^
  2 |
  3 | import Papa from "papaparse";
  4 | import type { RawSensorPacket } from "@/utils/api";
Next.js build worker exited with code: 1 and signal: null

Originator Checklist

  • Title matches the form "GHI # - []"
  • Target branch is correct
  • Unit Tests have been posted in issue <if applicable,can be super simple like a screenshot>
  • Issue has been linked to this PR
  • Changes generate no new warnings
    • merge main into your branch to resolve conflicts before opening PR!

Not all features introduced here have a relevant issue.
No unit test framework exists for this project, so unit tests have not been written.
Manually performed system & integration tests.


@ETSells
ETSells requested a review from favillat July 5, 2026 23:12
@ETSells ETSells changed the title GHI N/A - Fix API data pipeline & add altitude tape [FV/typescript-rewrite] GHI 112 - Fix API data pipeline & add altitude tape [FV/typescript-rewrite] Jul 6, 2026
@ETSells ETSells changed the title GHI 112 - Fix API data pipeline & add altitude tape [FV/typescript-rewrite] GHI #112 - Fix API data pipeline & add altitude tape [FV/typescript-rewrite] Jul 6, 2026
@ETSells ETSells linked an issue Jul 6, 2026 that may be closed by this pull request
@ETSells

ETSells commented Jul 11, 2026

Copy link
Copy Markdown
Member Author

Parent has been moved out of draft state and is ready for review. One minor tweak was needed here, but this is also still ready

@ETSells ETSells linked an issue Jul 20, 2026 that may be closed by this pull request
@ETSells ETSells changed the title GHI #112 - Fix API data pipeline & add altitude tape [FV/typescript-rewrite] GHI #112, #119 - Fix API data pipeline & add altitude tape [FV/typescript-rewrite] Jul 20, 2026
@ETSells
ETSells force-pushed the integration/state-estims-telemetry branch from f1f8190 to 621937c Compare August 17, 2026 19:51
@ETSells
ETSells requested review from favillat and removed request for favillat August 17, 2026 20:05
@ETSells

ETSells commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

@favillat Re-requesting review now that the base branch is ready to go.

@ETSells

ETSells commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

@NArmistead @MasterUser43 As this is top priority, I'm tagging both of you on the review as well. Please take a look and let me know if there's anything you don't feel you have sufficient expertise in.

Base automatically changed from FV/typescript-rewrite to main August 20, 2026 17:57
@favillat
favillat force-pushed the integration/state-estims-telemetry branch from 621937c to 0f55948 Compare August 20, 2026 17:57

@favillat favillat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, just a few minor considerations and a couple questions.

Comment thread src/utils/units/units.ts
METERS_TO_FEET = 3.280839895,
GRAVITY_MPS = 9.80665
}
export default ConversionFactors;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth converting this to const instead of an enum

export const ConversionFactors = {
    METERS_TO_FEET: 3.xx,
    GRAVITY_MPS: 9.xx,
} as const;

Comment thread src/utils/units/units.ts
*/
interface UnitsHandler {
/* Shared members for unitsHandler. Not typed for inheritance reasons. */
systemUnits: any

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious as to what inheritance reasons? we could try doing a generic type to make it generally safer.

break; /* SI/input units */

default:
console.error("Invalid altitude units.") /* intentional fallthrough to G */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming this was meant to be "Invalid acceleration units", further down there are a few more instances of this aswell

* @returns A string with the output value and its associated units
*/
getDisplayString(input: number): string {
if(this.systemUnits == AccelerationUnits.G_FORCE)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicky but it might be better to do strict equality checks here (===), they're faster and cleaner

}

/* Extra step for altitude: reference mode */
if(this.referenceElevation >= 0 && this.mode == AltitudeMode.QFE) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same strict equality check comment as before, just makes this more predictable and doesn't hurt

if (this.referenceElevation < 0) {
break; /* use QNH if no reference elevation defined. assumes we'd never launch from below sea level */
}
input -= this.referenceElevation; /* negative altitudes allowed */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This having no break here would mean any time we use QFE, we would default and log "Invalid altitude reference mode.". Instead why not skip the fall-thru all together and just let typescript types deal with issues? it might make future debugging a bit easier.

@NArmistead NArmistead left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not going to claim to understand everything that's going on but lgtm as far as I can tell

Comment thread src/utils/Three.tsx
Comment on lines +43 to +46
* Firmware uses the NED (North-East-Down) convention of X: roll,
* Y: pitch, Z: yaw, so at the identity orientation the model's
* nose must point along Three's +X (see baseQuat below). A 90°
* rotation on the +X axis aligns the firmware's body frame with

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ehh some of this comment is kinda redundant (i wrote it)

Comment thread src/utils/Three.tsx
* similarity transform (conjugation), since firmware's body-frame
* axes are not the same axes as Three's world frame axes. This is
* NOT the same as composing two rotations (simple multiplication) —
* conjugation is what's required when changing the basis a

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aren't we using the inverse here not the conjugate?

Comment thread src/utils/Three.tsx

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this working correctly now? i haven't tested on hardware since i fixed(?) it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Display unit conversion Integration: Upgrade to Quats

3 participants